test/metatx/ERC2771Forwarder.test.js (1)

231-243: Strengthen this atomicity test with post-revert state invariants.

This currently validates the revert reason, but not the “no side effects” guarantee (nonces/ETH state) after failure.

Suggested test hardening

       it('atomic batch with reverting request reverts the whole batch', async function () {
+        const noncesBefore = await Promise.all(
+          this.requests.map(request => this.forwarder.nonces(request.from)),
+        );
+        const forwarderBalanceBefore = await ethers.provider.getBalance(await this.forwarder.getAddress());
+
         // Add extra reverting request
         await this.forgeRequest(
           { value: 10n, data: this.receiver.interface.encodeFunctionData('mockFunctionRevertsNoReason') },
           this.accounts[requestCount],
         ).then(extraRequest => this.requests.push(extraRequest));
         // recompute total value with the extra request
         this.value = requestsValue(this.requests);

         await expect(
           this.forwarder.executeBatch(this.requests, ethers.ZeroAddress, { value: this.value }),
         ).to.be.revertedWithCustomError(this.forwarder, 'ERC2771ForwarderFailureInAtomicBatch');
+
+        expect(await ethers.provider.getBalance(await this.forwarder.getAddress())).to.equal(forwarderBalanceBefore);
+        for (const [i, request] of this.requests.slice(0, requestCount).entries()) {
+          expect(await this.forwarder.nonces(request.from)).to.equal(noncesBefore[i]);
+        }
       });

Based on learnings: in revert-semantics tests, also assert no ETH balance changes occurred.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/metatx/ERC2771Forwarder.test.js` around lines 231 - 243, Capture
pre-revert state (store balances and nonces) for the involved parties before
adding the reverting request — e.g., record ethers.provider.getBalance for the
sender(s), the forwarder contract, and receiver, and record per-sender nonce via
this.forwarder.getNonce(sender) or equivalent; then run the existing
expect(...).to.be.revertedWithCustomError(this.forwarder,
'ERC2771ForwarderFailureInAtomicBatch'); finally assert the post-revert balances
and nonces equal the pre-revert values (and that this.requests / this.value
didn’t change) to enforce no ETH or nonce side effects after executeBatch with
the reverting mockFunctionRevertsNoReason.

test/metatx/ERC2771Forwarder.test.js (1)

231-243: Strengthen this test with post-revert invariants (nonces + ETH).

Nice revert assertion. I’d also assert that signer nonces and forwarder ETH are unchanged after revert to directly lock in the audited regression behavior.

Suggested test hardening

 it('atomic batch with reverting request reverts the whole batch', async function () {
   // Add extra reverting request
   await this.forgeRequest(
     { value: 10n, data: this.receiver.interface.encodeFunctionData('mockFunctionRevertsNoReason') },
     this.accounts[requestCount],
   ).then(extraRequest => this.requests.push(extraRequest));
   // recompute total value with the extra request
   this.value = requestsValue(this.requests);
+
+  const noncesBefore = await Promise.all(this.requests.map(request => this.forwarder.nonces(request.from)));
+  const forwarderBalanceBefore = await ethers.provider.getBalance(this.forwarder);

   await expect(
     this.forwarder.executeBatch(this.requests, ethers.ZeroAddress, { value: this.value }),
   ).to.be.revertedWithCustomError(this.forwarder, 'ERC2771ForwarderFailureInAtomicBatch');
+
+  for (const [i, request] of this.requests.entries()) {
+    expect(await this.forwarder.nonces(request.from)).to.equal(noncesBefore[i]);
+  }
+  expect(await ethers.provider.getBalance(this.forwarder)).to.equal(forwarderBalanceBefore);
 });

Based on learnings: in revert-semantics tests, verify no balance effects occurred (e.g., negative balance-change assertions) to ensure no unintended state/value changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/metatx/ERC2771Forwarder.test.js` around lines 231 - 243, Before calling
executeBatch, record the current signer nonces for the involved accounts and the
forwarder contract ETH balance (use the same accounts from this.accounts and the
forwarder instance this.forwarder); after the
expect(...revertedWithCustomError(...)) assertion, re-query those nonces and the
forwarder balance and assert they are unchanged to ensure no state or value
changes occurred; locate the setup around the atomic batch test that builds
this.requests (including the extra request using mockFunctionRevertsNoReason)
and the compute of this.value via requestsValue to add these pre- and
post-revert invariants.